Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backports for 1.1.0-rc2 #30607

Merged
merged 11 commits into from
Jan 12, 2019
Merged

Backports for 1.1.0-rc2 #30607

merged 11 commits into from
Jan 12, 2019

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Jan 5, 2019

Backported PRs:

Need manual backport:

Non-merged PRs with backport label:

@KristofferC KristofferC added the DO NOT MERGE Do not merge this PR! label Jan 5, 2019
@ararslan
Copy link
Member

ararslan commented Jan 5, 2019

I've marked #30599 for backporting, which should fix the macOS CI. @staticfloat, it looks like we'll also need to backport whatever it is you did that fixed AppVeyor.

@staticfloat
Copy link
Member

So probably 8b189ec then

JeffBezanson and others added 6 commits January 9, 2019 11:25
This corresponds to macOS 10.12 Sierra. XCode 8 covers El Capitan and
Sierra, so if Travis is giving us XCode 8.x for x < 3, we're on El Cap.
Homebrew supports only three versions of macOS at a time, which means
that El Cap (10.11) is no longer support. This is likely why our Mac
builds are trying to build GCC from source; a bottle might not be
available.

(cherry picked from commit 862fe08)
PR #28478 moved the computation of the use counts before the finish call.
to fix #28444. However, the early parts of the finish call fixes up phi
node arguments, which fail to get counted if we look at use counts before
that fixup is performed. This causes #30594 where the only non-trivial use
is on the backedge of the phi and would thus incorrectly fail to get accounted
for. Fix that by taking the use count after phi fixup but before dce.

(cherry picked from commit f8f2045)
These changes cut some method dependency edges that tend to lead to
invalidations when loading packages.

(cherry picked from commit 13fc4c3)
SROA was accidentally treating a pending node as old and thus
getting the wrong type when querying the predecessor. As a result
it thought one of the paths was unreachable causing undefined data
to be introduced on that path (generally the `1.0` that happened to
already be in register).

Fix #29983

(cherry picked from commit da0179c)
I don't have concrete tests for these, but it looks like
they all need the `is_old` predicate for what they're doing,
so switch those over also while we're at it.

(cherry picked from commit 34f7a4a)
@KristofferC
Copy link
Member Author

@nanosoldier runbenchmarks("linalg", vs = ":release-1.1")

@KristofferC
Copy link
Member Author

@ararslan could you kick nanosoldier?

@staticfloat
Copy link
Member

The appveyor failure is bizarre; perl is complaining that it can't make changes without a backup here:

perl -i -ple 's/^\s*(EXTRALIB\s*\+=\s*-lSystemStubs)\s*$$/# $$1/g' $(dir $<)/Makefile.system

That line hasn't changed in 3 years. Could perl have changed out from underneath us?

@staticfloat
Copy link
Member

Also why are we even using perl here; we could just turn that into a normal .patch.

@ararslan
Copy link
Member

ararslan commented Jan 9, 2019

@nanosoldier runbenchmarks("linalg", vs=":release-1.1")

@nanosoldier

This comment has been minimized.

@vtjnash
Copy link
Member

vtjnash commented Jan 9, 2019

Also why are we even using perl here; we could just turn that into a normal .patch.

We could try, but this bug has been fixed since (at least) v0.1alpha2.5 over 7 years ago, so it would just be an empty file. (OpenMathLib/OpenBLAS@0d76196)

@KristofferC
Copy link
Member Author

@nanosoldier runbenchmarks(ALL, vs=":release-1.1")

@KristofferC
Copy link
Member Author

I tried to just remove it.

@@ -92,10 +92,6 @@ endif
# Do not overwrite the "-j" flag
OPENBLAS_BUILD_OPTS += MAKE_NB_JOBS=0

$(BUILDDIR)/$(OPENBLAS_SRC_DIR)/build-configured: $(BUILDDIR)/$(OPENBLAS_SRC_DIR)/source-extracted
perl -i -ple 's/^\s*(EXTRALIB\s*\+=\s*-lSystemStubs)\s*$$/# $$1/g' $(dir $<)/Makefile.system
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still need to create the build-configured file though; so just remove the perl invocation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, thanks.

@KristofferC KristofferC force-pushed the backport-1.1.0 branch 2 times, most recently from 6ee9441 to 17a5e1e Compare January 9, 2019 23:06
@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@KristofferC
Copy link
Member Author

AV seems to build OpenBLAS from scratch now and times out during test on 32 bit but fails on 64 bit with:

cp: cannot stat 'scratch/openblas-fd8d1868a126bb9f12bbc43b36ee30d1ba943fbb/libopenblas64_.dll': No such file or directory
make[1]: *** [/cygdrive/c/projects/julia/deps/blas.mk:112: /cygdrive/c/projects/julia/usr-staging/openblas-fd8d1868a126bb9f12bbc43b36ee30d1ba943fbb.tgz] Error 1
make[1]: Leaving directory '/cygdrive/c/projects/julia/deps'
make: *** [Makefile:57: julia-deps] Error 2

Did I do something bad in the last commit? Help plz.

@ararslan
Copy link
Member

PkgEval shows no regressions since RC1. Note that it was run prior to the most recent Makefile changes.

@staticfloat
Copy link
Member

I'm not sure how, but it looks like you only got part of 87c18d8. You need the rest of the changes as well.

@KristofferC
Copy link
Member Author

Well, that wasn't marked with backport label and it wasn't the commit in #30607 (comment) so it wasn't backported. I'll add that as well then.

But why do we need so many build backports to this branch? Did something change on AV's end?

@staticfloat
Copy link
Member

Well, that wasn't marked with backport label

Sorry about that.

But why do we need so many build backports to this branch? Did something change on AV's end?

Yes, the compiler versions changed, but also it's an improvement that reduces compile/test times, so it seemed like a good thing to include.

Improves #29867 by avoiding an invoke.

(cherry picked from commit f2d2d6f)
KristofferC and others added 3 commits January 11, 2019 17:13
(cherry picked from commit 2a2ba37)
* Auto-detect binarybuilder triplet

* Add OpenBLAS BinaryBuilder installation scaffolding

Also make it easier to add more BB-cached versions of dependencies in the future

* Enable `fixup-libgfortran.sh` to directly ask `$FC` for paths

* Tell Appveyor and Travis to use BinaryBuilder OpenBLAS

Also allow the build system to auto-guess the triplet

(cherry picked from commit 87c18d8)
* Fix `bb-install` naming conventions, add hashes

* Set `DEP_LIBS` to include `openblas` on Appveyor

* When guessing BB libc, default to `glibc` on Linux

* Fix bb-install bash parsing failure

Quote to avoid bash freaking out from spurious `)` from compilers that
have fancy version strings such as `(Red Hat 7.3.1-5)`, which gets
`lastword`'ed down to `7.3.1-5)`.

* Add `contrib/refresh_bb_tarballs.sh` to aid in batch-grabbing BB hashes

(cherry picked from commit 8b189ec)
@KristofferC
Copy link
Member Author

Backported those two but something is still wrong :(

make[1]: Leaving directory '/cygdrive/c/projects/julia'
make[1]: Entering directory '/cygdrive/c/projects/julia'
fatal: cannot change to '/cygdrive/c/projects/julia': No such file or directory

@staticfloat
Copy link
Member

I have no idea what Appveyor is smoking there. That doesn't make any sense to me, especially as we can see other processes happily going and entering that directory.

@KristofferC
Copy link
Member Author

KristofferC commented Jan 11, 2019

Ok, this happens on master as well. Doesn't stop the tests from running apparently.

@KristofferC KristofferC removed the DO NOT MERGE Do not merge this PR! label Jan 11, 2019
@ararslan ararslan merged commit 47038ea into release-1.1 Jan 12, 2019
@ararslan ararslan deleted the backport-1.1.0 branch January 12, 2019 20:03
vchuravy pushed a commit to JuliaLang/Distributed.jl that referenced this pull request Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants